refactor: optimize argocd-application-controller redis usage #5345
Conversation
… calls Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
|
Testing results on a busy Argo CD instance: The number of Redis calls reduced by ~30%:
Sent/received traffic reduced by almost 4x:
Controller memory usage before and after upgrade are the same:
|
| @@ -303,7 +303,8 @@ export class PodView extends React.Component<PodViewProps> { | |||
| renderMenu: () => this.props.nodeMenu(rnode) | |||
| }; | |||
| } | |||
|
|
|||
| }); | |||
| (tree.nodes || []).forEach((rnode: ResourceTreeNode) => { | |||
There was a problem hiding this comment.
Oh, yes. It appears that UI might break if pod appears before the parent resource in ApplicationTree.Nodes list. After introducing node sorting in backend this reproduces consistently.
util/cache/inmemory.go
Outdated
| err := gob.NewEncoder(&buf).Encode(obj) | ||
| if err != nil { | ||
| return false | ||
| } |
There was a problem hiding this comment.
Could it be that we'll silently drop some real errors here? In which cases does gob.NewEncoder() fail?
There was a problem hiding this comment.
Agree, changed method so it returns an error instead of silently dropping it
util/cache/inmemory.go
Outdated
| if !found { | ||
| return false | ||
| } | ||
| existingBuf := bufIf.(bytes.Buffer) |
There was a problem hiding this comment.
Maybe we should safeguard this typecast, but I'm not sure. Can there be something else than bytes.Buffer in the in-memory cache for whatever reason?
There was a problem hiding this comment.
I think we want to panic here, because if In-memory cache has garbage we should restart pod. Added explicit check and panic with the meaningful message
util/cache/twolevelclient.go
Outdated
|
|
||
| type twoLevelClient struct { | ||
| inMemoryCache *InMemoryCache | ||
| client CacheClient |
There was a problem hiding this comment.
I think it would be great to have some consistency in the member names, i.e. inMemoryCache and externalCache. But this is just nitpicking.
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
alexmt
force-pushed
the
two-level-caching
branch
from
327ce7d to
d12fe5a
Compare
alexmt
added
the
cherry-pick/1.8
|
Cherry picking to 1.8 since it is fixing unexpected traffic increase caused by #4965 |
* refactor: controller uses two level caching to reduce number of redis calls Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
…j#5345) * refactor: controller uses two level caching to reduce number of redis calls Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Signed-off-by: Alexander Matyushentsev AMatyushentsev@gmail.com
The
argocd-application-controlleruses Redis a LOT that results in significant traffic that might be costly in the cloud. PR introduces two-level caching: before reading/writing to redis controller check in-memory cache first and don't use redis if in-memory cache already has required entry. This is safe since the controller is the only writer to the Redis.